Conversation
Contributor
WalkthroughAdded Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant App as Application
participant Registry as DotrainRegistry
participant YAML as OrderbookYaml
participant Remote as Remote Token API
participant Chain as Blockchain (ERC20)
User->>App: Request token list
App->>Registry: getOrderbookYaml()
Registry->>YAML: construct OrderbookYaml(settings)
YAML-->>Registry: OrderbookYaml
Registry-->>App: OrderbookYaml
App->>YAML: getTokens()
alt using-tokens-from configured
YAML->>Remote: fetch_tokens(urls)
Remote-->>YAML: remote token cfgs
YAML->>YAML: merge remote with local token cfgs
end
loop per token with missing fields
YAML->>Chain: ERC20 token_info / decimals / name / symbol
Chain-->>YAML: on-chain token metadata
end
YAML->>App: Vec(ExtendedTokenInfo)
App-->>User: Display tokens (include chain_id, logo_uri)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The TokenInfoExtended struct uses serde rename_all = "camelCase", so the JS property is chainId not chain_id.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/js_api/src/gui/select_tokens.rs (1)
338-348: Silent error suppression in token fetching.Failed token fetches are silently filtered out. While this is existing behavior and provides graceful degradation, users won't know if specific tokens failed to load.
Consider whether logging or surfacing partial failures would be beneficial for debugging. This is a minor observability concern and not blocking.
🤖 Fix all issues with AI agents
In `@crates/js_api/src/yaml/mod.rs`:
- Around line 618-646: The test test_get_tokens_remote_fetch_failure currently
only asserts that get_tokens() returned an Err; update it to assert the specific
error variant and/or message from OrderbookYaml::get_tokens so we ensure the
failure is the expected remote-fetch error (not some unrelated failure). Replace
the generic assert!(result.is_err()) with code that matches the Err value (e.g.,
unwrap_err() or match on the Result) and assert the error is the expected enum
variant or contains the expected remote-fetch text returned by the HTTP 500
response.
0xgleb
approved these changes
Jan 28, 2026
| #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] | ||
| #[cfg_attr(target_family = "wasm", derive(Tsify))] | ||
| #[serde(rename_all = "camelCase")] | ||
| pub struct TokenInfoExtended { |
Collaborator
There was a problem hiding this comment.
nitpick: ExtendedTokenInfo might read more naturally
hardyjosh
requested changes
Jan 28, 2026
| unchecked_return_type = "TokenInfoExtended[]", | ||
| return_description = "Array of token information" | ||
| )] | ||
| pub async fn get_tokens(&mut self) -> Result<Vec<TokenInfoExtended>, OrderbookYamlError> { |
Contributor
There was a problem hiding this comment.
Should this logic be in js_api or somewhere common?
Collaborator
Author
There was a problem hiding this comment.
Added the implementation to erc20 file and refactored other places to use this new helper
Add ExtendedTokenInfo::from_token_cfg() method to centralize the logic for converting TokenCfg to ExtendedTokenInfo, fetching missing fields (decimals, label, symbol) from RPC when needed. - Add from_token_cfg method to ExtendedTokenInfo in erc20.rs - Update yaml/mod.rs get_tokens() to use new method instead of erroring - Simplify gui/mod.rs get_token_info() to use new method - Simplify gui/select_tokens.rs get_all_tokens() to use new method - Update tests to reflect new behavior (tries RPC instead of erroring)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/js_api/src/gui/select_tokens.rs (1)
306-315: Consider logging errors instead of silently dropping them.The
filter_mapsilently discards tokens that fail to fetch. While this prevents a single failing token from breaking the entire list, it may hide issues (e.g., RPC failures, invalid token configurations) that could be useful for debugging.Consider adding logging for failed token fetches:
♻️ Suggested improvement
let mut results: Vec<ExtendedTokenInfo> = futures::stream::iter(fetch_futures) .buffer_unordered(MAX_CONCURRENT_FETCHES) .filter_map(|res| async { match res { Ok(info) => Some(info), - Err(_) => None, + Err(e) => { + tracing::warn!("Failed to fetch token info: {}", e); + None + } } }) .collect() .await;
🤖 Fix all issues with AI agents
In `@crates/js_api/src/yaml/mod.rs`:
- Around line 164-165: The MissingField(String) error variant is unused and
should be removed from the error enum to avoid dead code; update the enum by
deleting the MissingField variant and its #[error(...)] annotation, and ensure
no other code references MissingField (verify ExtendedTokenInfo::from_token_cfg
and places that construct errors use ERC20Error or other existing variants
instead); run cargo build/tests to confirm no remaining references.
- Around line 149-152: The loop awaits ExtendedTokenInfo::from_token_cfg
sequentially causing O(n) network round-trips; instead spawn all futures and
await them in parallel (e.g., map tokens.values() to futures by calling
ExtendedTokenInfo::from_token_cfg(token) without awaiting, collect into a Vec<_>
and use futures::future::try_join_all or futures::stream::FuturesUnordered to
run them concurrently), then collect the Results into token_infos; ensure you
add the needed futures import and propagate any error handling the same way as
the current await.
In `@packages/orderbook/test/js_api/orderbookYaml.test.ts`:
- Around line 193-206: The test hardcodes port 8087 when starting the mock
server which can cause port conflicts; change the beforeAll/start logic to start
the mockttp server without a port (call mockServer.start() with no args) so it
dynamically allocates a free port, capture the base URL from mockServer.url (or
equivalent) into a serverUrl variable, and update any places in the test and
referenced YAML to use this serverUrl instead of "http://localhost:8087" (adjust
getTokens tests and any YAML-loading logic to substitute serverUrl at runtime).
Ensure beforeAll uses mockServer.start(), store mockServer.url, and that any
hardcoded URL strings are replaced with the dynamic serverUrl.
Comment on lines
+149
to
+152
| let mut token_infos: Vec<ExtendedTokenInfo> = Vec::new(); | ||
| for token in tokens.values() { | ||
| token_infos.push(ExtendedTokenInfo::from_token_cfg(token).await?); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Sequential token processing may be slow for large token lists.
Each ExtendedTokenInfo::from_token_cfg call is awaited individually in a loop. If tokens have missing fields requiring RPC calls, this becomes O(n) sequential network requests.
♻️ Consider parallel token processing with futures::join_all
+use futures::future::join_all;
+
let tokens = self.yaml.get_tokens()?;
- let mut token_infos: Vec<ExtendedTokenInfo> = Vec::new();
- for token in tokens.values() {
- token_infos.push(ExtendedTokenInfo::from_token_cfg(token).await?);
- }
+ let token_futures: Vec<_> = tokens
+ .values()
+ .map(|token| ExtendedTokenInfo::from_token_cfg(token))
+ .collect();
+
+ let results: Vec<Result<ExtendedTokenInfo, _>> = join_all(token_futures).await;
+ let token_infos: Vec<ExtendedTokenInfo> = results
+ .into_iter()
+ .collect::<Result<Vec<_>, _>>()?;
Ok(token_infos)🤖 Prompt for AI Agents
In `@crates/js_api/src/yaml/mod.rs` around lines 149 - 152, The loop awaits
ExtendedTokenInfo::from_token_cfg sequentially causing O(n) network round-trips;
instead spawn all futures and await them in parallel (e.g., map tokens.values()
to futures by calling ExtendedTokenInfo::from_token_cfg(token) without awaiting,
collect into a Vec<_> and use futures::future::try_join_all or
futures::stream::FuturesUnordered to run them concurrently), then collect the
Results into token_infos; ensure you add the needed futures import and propagate
any error handling the same way as the current await.
Comment on lines
+164
to
+165
| #[error("Missing required field: {0}")] | ||
| MissingField(String), |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "MissingField" --type rustRepository: rainlanguage/rain.orderbook
Length of output: 1165
🏁 Script executed:
# Check the file to understand the error enum and its context
cat -n crates/js_api/src/yaml/mod.rs | head -200Repository: rainlanguage/rain.orderbook
Length of output: 9155
🏁 Script executed:
# Search for ExtendedTokenInfo and from_token_cfg to understand error handling
rg -n "ExtendedTokenInfo|from_token_cfg" --type rust -A 5Repository: rainlanguage/rain.orderbook
Length of output: 7680
Remove the unused MissingField error variant.
The MissingField variant is defined with a readable message handler but is never constructed in this module. Error handling in ExtendedTokenInfo::from_token_cfg (called at line 151) flows through the ERC20Error variant instead of MissingField.
🤖 Prompt for AI Agents
In `@crates/js_api/src/yaml/mod.rs` around lines 164 - 165, The
MissingField(String) error variant is unused and should be removed from the
error enum to avoid dead code; update the enum by deleting the MissingField
variant and its #[error(...)] annotation, and ensure no other code references
MissingField (verify ExtendedTokenInfo::from_token_cfg and places that construct
errors use ERC20Error or other existing variants instead); run cargo build/tests
to confirm no remaining references.
Comment on lines
+193
to
+206
| describe('getTokens tests', async function () { | ||
| const mockServer = getLocal(); | ||
|
|
||
| beforeAll(async () => { | ||
| await mockServer.start(8087); | ||
| }); | ||
|
|
||
| afterAll(async () => { | ||
| await mockServer.stop(); | ||
| }); | ||
|
|
||
| beforeEach(async () => { | ||
| await mockServer.reset(); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using dynamic port allocation to avoid port conflicts.
The test uses hardcoded port 8087, which could cause conflicts if tests run in parallel or if the port is already in use. The mockttp library supports dynamic port allocation.
♻️ Optional: Use dynamic port allocation
describe('getTokens tests', async function () {
const mockServer = getLocal();
+ let serverUrl: string;
beforeAll(async () => {
- await mockServer.start(8087);
+ await mockServer.start();
+ serverUrl = mockServer.url;
});Then update YAML references to use serverUrl instead of hardcoded http://localhost:8087.
🤖 Prompt for AI Agents
In `@packages/orderbook/test/js_api/orderbookYaml.test.ts` around lines 193 - 206,
The test hardcodes port 8087 when starting the mock server which can cause port
conflicts; change the beforeAll/start logic to start the mockttp server without
a port (call mockServer.start() with no args) so it dynamically allocates a free
port, capture the base URL from mockServer.url (or equivalent) into a serverUrl
variable, and update any places in the test and referenced YAML to use this
serverUrl instead of "http://localhost:8087" (adjust getTokens tests and any
YAML-loading logic to substitute serverUrl at runtime). Ensure beforeAll uses
mockServer.start(), store mockServer.url, and that any hardcoded URL strings are
replaced with the dynamic serverUrl.
Collaborator
Author
There was a problem hiding this comment.
This is fine for now.
Contributor
There was a problem hiding this comment.
@findolor, understood! I'll leave the hardcoded port approach as-is for now.
🧠 Learnings used
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1974
File: packages/ui-components/src/__tests__/DeploymentSteps.test.ts:123-126
Timestamp: 2025-07-09T12:35:45.699Z
Learning: In packages/ui-components/src/__tests__/DeploymentSteps.test.ts, findolor prefers to keep mock initializations (like setSelectToken) in individual test cases rather than consolidating them into shared beforeEach blocks, even when it results in duplication.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1891
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/page.test.ts:66-80
Timestamp: 2025-06-08T18:43:51.842Z
Learning: In the rain.orderbook webapp test files, when mocking objects like the transaction manager, it's acceptable to use simple empty objects with ts-expect-error comments rather than providing complete mock implementations with all properties and methods.
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1597
File: packages/ui-components/src/__tests__/OrderDetail.test.ts:120-120
Timestamp: 2025-04-08T09:18:46.653Z
Learning: In test files for the Rain Orderbook project, it's acceptable to bypass TypeScript's strict typing using constructs like `as unknown as [Type]` to create simplified mock objects that don't need to implement the entire interface.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2117
File: packages/ui-components/src/__tests__/VaultIdInformation.test.ts:9-13
Timestamp: 2025-09-02T08:04:44.814Z
Learning: In packages/ui-components/src/__tests__/VaultIdInformation.test.ts and similar test files in the rain.orderbook project, the passthrough vi.mock('rainlanguage/orderbook', async (importOriginal) => { return { ...(await importOriginal()) }; }); block is required for tests to run properly, even when not overriding any exports. This is needed due to the specific Vitest configuration or test environment setup in the project.
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2083
File: packages/ui-components/src/__tests__/VaultsListTable.test.ts:16-19
Timestamp: 2025-08-14T18:29:32.933Z
Learning: In the rain.orderbook project's UI components tests, mocking hooks like useToasts is often required as infrastructure even when not directly asserting on their calls, because components internally depend on these hooks. Removing such mocks would break component rendering in tests and require more complex test setup with providers.
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-28T08:52:27.729Z
Learning: Applies to packages/orderbook/**/*.{ts,tsx,js,jsx} : For Orderbook TypeScript package, run tests with `nix develop -c npm run test -w rainlanguage/orderbook`
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1907
File: packages/orderbook/test/common/test.test.ts:75-77
Timestamp: 2025-06-04T10:21:01.388Z
Learning: The DotrainOrder.create API in packages/orderbook/test/common/test.test.ts is internal and not used directly in consumer applications, so API changes here don't require external breaking change documentation.
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1559
File: packages/ui-components/src/__tests__/OrderOrVaultHash.test.ts:94-94
Timestamp: 2025-04-04T11:25:21.518Z
Learning: In the rain.orderbook project, minimal test fixtures are preferred over complete mocks that implement the entire interface. Type casting (e.g., `as unknown as SgVault`) is an acceptable approach to maintain both minimal fixtures and TypeScript type compatibility.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1903
File: tauri-app/src/routes/orders/[network]-[orderHash]/page.test.ts:68-69
Timestamp: 2025-06-17T16:28:19.664Z
Learning: In the tauri-app test files, the team is comfortable using single URLs in rpcs arrays for test mocks rather than multiple URLs, even when testing multi-RPC functionality. They don't consider this a coverage gap that needs to be addressed.
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 1911
File: crates/subgraph/src/vault_balance_changes_query.rs:133-135
Timestamp: 2025-07-23T13:03:40.669Z
Learning: User 0xgleb prefers not to make changes to test code when tests are already passing, even if the code format could be standardized differently. Working test code should be left alone unless there's a functional issue.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1956
File: packages/ui-components/src/__tests__/VaultDetail.test.ts:177-181
Timestamp: 2025-07-04T09:14:35.748Z
Learning: In the rain.orderbook project, maintainer findolor is fine with mutating mock data directly in test files using ts-expect-error comments, preferring this pragmatic approach over immutable patterns like spread operators for test data updates.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1996
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts:303-303
Timestamp: 2025-07-17T10:35:09.329Z
Learning: In packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts, findolor is fine with hardcoded 2-second delays using setTimeout in test cases for waiting after token selection, preferring this approach over deterministic waiting patterns.
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1700
File: tauri-app/src/lib/mocks/mockConfigSource.ts:6-6
Timestamp: 2025-04-28T10:58:11.124Z
Learning: In mock data for testing in this codebase, it's acceptable to use URL placeholders like 'https://mainnet.infura.io/v3/YOUR-PROJECT-ID' as they clearly indicate the expected format for actual implementation.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1916
File: packages/ui-components/src/lib/__fixtures__/settings-12-11-24.json:182-195
Timestamp: 2025-06-10T12:04:54.107Z
Learning: In test fixture files like `packages/ui-components/src/lib/__fixtures__/settings-12-11-24.json`, network configuration inconsistencies (such as matchain using Polygon's RPC, chainId, and currency while having its own network key) are acceptable since they are used for testing purposes only.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2099
File: .env.example:2-2
Timestamp: 2025-08-26T10:18:39.834Z
Learning: In the rain.orderbook project, test API tokens may be committed to .env.example files during development. The maintainer findolor is comfortable revoking test tokens when flagged, but the placeholder approach with guidance comments is still the recommended best practice.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1983
File: crates/js_api/src/gui/deposits.rs:147-156
Timestamp: 2025-07-11T06:41:11.924Z
Learning: In the Rain OrderBook project, token info is already cached in the YAML file which serves as the source of truth, so the `get_token_info()` async method in the GUI SDK retrieves from pre-loaded/parsed data rather than making expensive network calls.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2044
File: tauri-app/src/routes/orders/[chainId]-[orderbook]-[orderHash]/+page.svelte:76-78
Timestamp: 2025-07-31T19:03:56.594Z
Learning: In the rainlanguage/rain.orderbook project, when the Tauri app has issues preventing proper testing, the team prefers to defer Tauri-specific implementations to separate PRs rather than including untested code, especially for financial operations like withdrawals.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2001
File: crates/common/src/raindex_client/order_quotes.rs:62-69
Timestamp: 2025-07-16T14:33:13.457Z
Learning: In the rainlanguage/rain.orderbook codebase, findolor considers hardcoded decimal values (18 and 36) in order quote formatting logic to be acceptable for their use case, even when dynamic token decimals could theoretically provide more accurate formatting for different tokens.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR `#1884` and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2163
File: packages/webapp/src/routes/deploy/[orderName]/[deploymentKey]/fullDeployment.test.ts:214-215
Timestamp: 2025-09-24T11:06:36.718Z
Learning: In the rain.orderbook project, findolor is comfortable with public token addresses in test files even when they trigger Gitleaks warnings, as these are legitimate public addresses and not actual secrets requiring annotation or refactoring.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR `#1884` in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 1957
File: packages/ui-components/src/lib/components/tables/OrdersListTable.svelte:86-94
Timestamp: 2025-06-27T18:41:18.194Z
Learning: In the rain.orderbook UI components, the tokensQuery should use `enabled: true` rather than `Object.keys($activeSubgraphs).length > 0` because tokens are needed for filtering functionality even when no specific subgraphs are selected. The multiSubgraphArgs falls back to default subgraphs from settings, so disabling the query when no subgraphs are active would break the token filtering feature.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR `#1884` in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
hardyjosh
approved these changes
Jan 28, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@crates/js_api/ARCHITECTURE.md`:
- Around line 133-142: The docs incorrectly state OrderbookYaml.getTokens()
returns TokenInfo[]; update the documentation to reflect the correct return type
ExtendedTokenInfo[] and mention it returns all tokens (with chain_id, address,
decimals, symbol, name plus any extended fields) and still asynchronously
fetches remote tokens from using-tokens-from URLs; update the entry under `yaml
(src/yaml/mod.rs)` and the export line `OrderbookYaml.getTokens() ->
TokenInfo[]` to `OrderbookYaml.getTokens() -> ExtendedTokenInfo[]` so it matches
the actual API surface for OrderbookYaml and ExtendedTokenInfo.
In `@packages/orderbook/README.md`:
- Line 596: Update the README bullet for OrderbookYaml.getTokens() to state that
it returns an array of ExtendedTokenInfo (not a generic token shape) and list
the exact fields included such as key, chain_id, address, decimals, symbol, name
and logo_uri; also clarify that remote tokens fetched via using-tokens-from URLs
are merged/returned as ExtendedTokenInfo. Ensure you reference
OrderbookYaml.getTokens() and ExtendedTokenInfo in the text so readers know the
exact type returned.
- Around line 412-426: Update the README example to reflect the new return type
from OrderbookYaml.getTokens(): replace the comment saying "TokenInfo[] with
chain_id, address, decimals, symbol, name" with "ExtendedTokenInfo[] including
key, chain_id, address, decimals, symbol, name, and optional logo_uri", and
ensure any mention of TokenInfo in the example or surrounding text (e.g., the
getOrderbookYaml()/getTokens() example) is changed to ExtendedTokenInfo and
documents the added key and optional logo_uri fields.
Comment on lines
+133
to
+142
| - `getOrderbookYaml() -> OrderbookYaml` → returns an `OrderbookYaml` instance from the registry's shared settings YAML for querying tokens, networks, orderbooks, etc. | ||
| - `getGui(orderKey, deploymentKey, serializedState?, stateCallback?)` → merge `settings + order`, optionally restore serialized state, and produce a `DotrainOrderGui` instance. | ||
| - Errors: `DotrainRegistryError` covers fetch/parse/HTTP/URL issues and wraps `GuiError`. Also returns human-readable messages. | ||
|
|
||
| - `yaml` (src/yaml/mod.rs) | ||
| - Purpose: Wasm-friendly wrapper around orderbook YAML parsing to retrieve an `OrderbookCfg` by contract address. | ||
| - Purpose: Wasm-friendly wrapper around orderbook YAML parsing to retrieve configuration objects by address or query token metadata. | ||
| - Exports: | ||
| - `OrderbookYaml.new([yamlSources], validate?) -> OrderbookYaml`: parse/merge/optionally validate sources. | ||
| - `OrderbookYaml.getOrderbookByAddress(address) -> OrderbookCfg`. | ||
| - `OrderbookYaml.getTokens() -> TokenInfo[]` (async): returns all tokens from YAML with `chain_id`, `address`, `decimals`, `symbol`, and `name`. Automatically fetches remote tokens from `using-tokens-from` URLs. |
Contributor
There was a problem hiding this comment.
Fix the documented return type for getTokens().
This should be ExtendedTokenInfo[] now.
✅ Suggested doc fix
- - `OrderbookYaml.getTokens() -> TokenInfo[]` (async): returns all tokens from YAML with `chain_id`, `address`, `decimals`, `symbol`, and `name`. Automatically fetches remote tokens from `using-tokens-from` URLs.
+ - `OrderbookYaml.getTokens() -> ExtendedTokenInfo[]` (async): returns all tokens from YAML with `key`, `chain_id`, `address`, `decimals`, `symbol`, `name` (and optional `logo_uri`). Automatically fetches remote tokens from `using-tokens-from` URLs.🤖 Prompt for AI Agents
In `@crates/js_api/ARCHITECTURE.md` around lines 133 - 142, The docs incorrectly
state OrderbookYaml.getTokens() returns TokenInfo[]; update the documentation to
reflect the correct return type ExtendedTokenInfo[] and mention it returns all
tokens (with chain_id, address, decimals, symbol, name plus any extended fields)
and still asynchronously fetches remote tokens from using-tokens-from URLs;
update the entry under `yaml (src/yaml/mod.rs)` and the export line
`OrderbookYaml.getTokens() -> TokenInfo[]` to `OrderbookYaml.getTokens() ->
ExtendedTokenInfo[]` so it matches the actual API surface for OrderbookYaml and
ExtendedTokenInfo.
Comment on lines
+412
to
+426
| The SDK merges the shared settings YAML with each order's `.rain` content before you ever build a GUI. | ||
|
|
||
| #### Access tokens from registry settings | ||
|
|
||
| Use `getOrderbookYaml()` to access the shared settings as an `OrderbookYaml` instance, then query tokens, networks, or orderbooks: | ||
|
|
||
| ```ts | ||
| const orderbookYamlResult = registry.getOrderbookYaml(); | ||
| if (orderbookYamlResult.error) throw new Error(orderbookYamlResult.error.readableMsg); | ||
| const orderbookYaml = orderbookYamlResult.value; | ||
|
|
||
| const tokensResult = await orderbookYaml.getTokens(); | ||
| if (tokensResult.error) throw new Error(tokensResult.error.readableMsg); | ||
| const tokens = tokensResult.value; // TokenInfo[] with chain_id, address, decimals, symbol, name | ||
| ``` |
Contributor
There was a problem hiding this comment.
Update the example type to ExtendedTokenInfo (and include key/optional logo_uri).
The API now returns ExtendedTokenInfo[], not TokenInfo[], and it includes key plus optional logo_uri. This doc line will mislead SDK users.
✅ Suggested doc fix
-const tokens = tokensResult.value; // TokenInfo[] with chain_id, address, decimals, symbol, name
+const tokens = tokensResult.value; // ExtendedTokenInfo[] with key, chain_id, address, decimals, symbol, name (logo_uri optional)🤖 Prompt for AI Agents
In `@packages/orderbook/README.md` around lines 412 - 426, Update the README
example to reflect the new return type from OrderbookYaml.getTokens(): replace
the comment saying "TokenInfo[] with chain_id, address, decimals, symbol, name"
with "ExtendedTokenInfo[] including key, chain_id, address, decimals, symbol,
name, and optional logo_uri", and ensure any mention of TokenInfo in the example
or surrounding text (e.g., the getOrderbookYaml()/getTokens() example) is
changed to ExtendedTokenInfo and documents the added key and optional logo_uri
fields.
|
|
||
| - `getOrderHash`, `keccak256`, `keccak256HexString` – deterministic hashing helpers for Rain orders or arbitrary payloads. | ||
| - `Float` – arbitrary-precision arithmetic with parsing, formatting, comparisons, math ops, fixed-decimal conversions, and helpers like `Float.zero()` or `.formatWithRange(...)`. | ||
| - `OrderbookYaml.getTokens()` – async method returning all tokens from YAML configuration with `chain_id`, `address`, `decimals`, `symbol`, and `name`. Automatically fetches remote tokens from `using-tokens-from` URLs. |
Contributor
There was a problem hiding this comment.
Align utility export wording with ExtendedTokenInfo.
The bullet still implies a generic token info shape; mention ExtendedTokenInfo (key/chain_id/logo_uri) for accuracy.
✅ Suggested doc fix
-- `OrderbookYaml.getTokens()` – async method returning all tokens from YAML configuration with `chain_id`, `address`, `decimals`, `symbol`, and `name`. Automatically fetches remote tokens from `using-tokens-from` URLs.
+- `OrderbookYaml.getTokens()` – async method returning `ExtendedTokenInfo[]` (includes `key`, `chain_id`, `address`, `decimals`, `symbol`, `name`, optional `logo_uri`). Automatically fetches remote tokens from `using-tokens-from` URLs.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - `OrderbookYaml.getTokens()` – async method returning all tokens from YAML configuration with `chain_id`, `address`, `decimals`, `symbol`, and `name`. Automatically fetches remote tokens from `using-tokens-from` URLs. | |
| - `OrderbookYaml.getTokens()` – async method returning `ExtendedTokenInfo[]` (includes `key`, `chain_id`, `address`, `decimals`, `symbol`, `name`, optional `logo_uri`). Automatically fetches remote tokens from `using-tokens-from` URLs. |
🤖 Prompt for AI Agents
In `@packages/orderbook/README.md` at line 596, Update the README bullet for
OrderbookYaml.getTokens() to state that it returns an array of ExtendedTokenInfo
(not a generic token shape) and list the exact fields included such as key,
chain_id, address, decimals, symbol, name and logo_uri; also clarify that remote
tokens fetched via using-tokens-from URLs are merged/returned as
ExtendedTokenInfo. Ensure you reference OrderbookYaml.getTokens() and
ExtendedTokenInfo in the text so readers know the exact type returned.
Contributor
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
Contributor
|
The author of this PR is on the CodeRabbit Free Plan. In order to use the Chat feature, please upgrade the PR author to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
See issues:
The SDK previously lacked a convenient way to query token metadata from YAML configuration. Users needed to work through the GUI layer or manually parse YAML to access token information. Additionally, there was no direct way to access the shared
OrderbookYamlinstance from aDotrainRegistryfor querying networks, tokens, and orderbooks.This PR addresses these gaps by:
TokenInfoExtendedtype that includeschain_idalongside other token metadataOrderbookYaml.getTokens()for fetching all tokens including remote tokens fromusing-tokens-fromURLsDotrainRegistry.getOrderbookYaml()to access the shared settings YAML instanceSolution
New Shared Type:
TokenInfoExtendedIntroduced a new struct in
crates/common/src/erc20.rsthat consolidates token information:key,address,decimals,name,symbol,chain_idTokenInfostruct that was defined in the GUI modulegetTokenInfo,getAllTokenInfos,getAllTokens) and the new YAML methodsNew Method:
OrderbookYaml.getTokens()TokenInfoExtended[]using-tokens-fromURLs if configuredNew Method:
DotrainRegistry.getOrderbookYaml()OrderbookYamlinstance from the registry's shared settingsUI Component Updates
Updated Svelte components to use
TokenInfoExtended:SelectToken.svelteTokenSelectionModal.svelteDeploymentSteps.svelteTokenSelectionModal.test.tsDocumentation
Updated architecture and README documentation to reflect the new APIs.
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.